Skip to content

refactor: BIPS-38082 share AuthenticationObj across muxed providers#196

Merged
btfhernandez merged 2 commits into
mainfrom
refactor/BIPS-38082-share-authenticationObj-across-muxed-providers
Jun 1, 2026
Merged

refactor: BIPS-38082 share AuthenticationObj across muxed providers#196
btfhernandez merged 2 commits into
mainfrom
refactor/BIPS-38082-share-authenticationObj-across-muxed-providers

Conversation

@btfhernandez
Copy link
Copy Markdown
Contributor

@btfhernandez btfhernandez commented May 29, 2026

Cherry-picked from #194.

Purpose of the PR

  • Replace per-operation sign-in/sign-out calls with a single shared AuthenticationObj that both muxed providers (framework + sdkv2) receive at configure time and hold for the entire plugin lifetime.

Linked JIRA issue(s)

Summary of changes

New providers/utils/session.go introduces three functions that own the shared session lifetime:

  • InitSharedAuth(cacheKey, build) — lazily builds the AuthenticationObj, calls GetPasswordSafeAuthentication once, and caches both the object and the SignAppinResponse. Subsequent callers with the same key hit the cache; a changed key (e.g., acceptance tests rotating mock servers) signs the old session out first.
  • ShutdownSharedAuth() — called from main() after tf5server.Serve returns; signs out and clears the cache. Safe to call multiple times.
  • ResetSharedAuthForTest() — test-only helper to clear cache state between acceptance-test cases without making an HTTP round-trip.

providers/utils/methods.go — removed ~77 lines: the package-level globals SignInCount, AuthMu, signAppinResponse, and the Authenticate, SignOut, DeleteAssetByID functions that used them. The refcount-based session management is no longer needed.

main.go — imports utils and calls utils.ShutdownSharedAuth() between tf5server.Serve returning and log.Fatal, because log.Fatal calls os.Exit and would skip any deferred cleanup.

providers/provider_sdkv2/provider.go and providers/provider_framework/provider.go — both providerConfigure/Configure functions now call InitSharedAuth with a deterministic cache key (pipe-joined url|apiVersion|apiKey|clientId|clientSecret|accountName|verifyca|certName). Both providers produce the same key from identical provider-block config, so the second one to configure hits the cache and pays no network cost.

providers/provider_sdkv2/common.go — removed the authenticate() and signOut() wrapper functions; introduced providerMeta struct holding authObj *auth.AuthenticationObj and signAppin entities.SignAppinResponse. providerConfigure now returns *providerMeta instead of a bare *AuthenticationObj.

All sdkv2 resources (folders.go, safes.go, managed_account.go, secrets.go) — replaced m.(*auth.AuthenticationObj) casts with m.(*providerMeta) and removed every authenticate(d, m) / signOut(d, m) call pair.

All framework resources and data sources (assets_resource.go, databases_resource.go, workgroups_resource.go, functional_accounts_resource.go, managed_account_ephemeral.go, secrets_ephemeral.go, and all *_datasource.go files) — removed the per-handler utils.Authenticate + deferred utils.SignOut blocks. The APISignOut helper in managed_systems_by_database_resource.go is also deleted.

assets_resource.goDeleteAssetByID (the utils helper that wrapped Authenticate+SignOut) replaced with an inline assets.NewAssetObj + assetObj.DeleteAssetById call, since auth and signout are no longer the caller's responsibility.

providers/utils/methods_test.go — rewrote the auth/session test suite against the new API surface (TestInitSharedAuth_*, TestShutdownSharedAuth_*); removed TestAuthenticate, TestSignOut, TestSignOutError, TestDeleteAssetByID. Net result: ~363 lines vs ~600 before.

providers/provider_sdkv2/provider_test.go — added newSignInMockServer helper and utils.ResetSharedAuthForTest() calls so TestProviderConfigureWithApiKey / TestProviderConfigureWithCredentials can exercise the full providerConfigure path end-to-end. Tests now point at a real TLS mock server instead of the fake constant URL.

All sdkv2 test files (folders_safes_test.go, managed_account_test.go, secrets_test.go) — updated call sites from authenticate (bare *AuthenticationObj) to &providerMeta{authObj: authenticate}.

Net change: -519 lines across 33 files.

Checklist

Release

  • Priority release required due to Hot fix / Escalation / Critical bug
  • Priority release not required (can be released later with other stories or bugs fixes)

Testing

  • dev
  • automation (required for feature branch merges and significant changes)
  • manual (required for feature branch merges and significant changes)

Automation

  • no changes are required
  • existing tests have been updated
  • new tests have been added
  • further changes are required by the automation team

One signin in Configure, one signout in main, shared cookie jar. Fixes 401s caused by framework and sdkv2 each owning their own jar.
@btfhernandez btfhernandez marked this pull request as ready for review May 29, 2026 00:47
@btfhernandez btfhernandez requested a review from a team as a code owner May 29, 2026 00:47
Copilot AI review requested due to automatic review settings May 29, 2026 00:47
@btfhernandez btfhernandez marked this pull request as draft May 29, 2026 00:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the muxed provider to share a single AuthenticationObj (and session/cookie jar) across both the framework and SDKv2 provider implementations. Previously each provider built its own auth and a package-level refcount tried to share signin/signout calls but not the underlying HTTP/cookie state, causing one provider to make API calls against an invalidated cookie. The new approach lazily builds and caches the auth object in Configure keyed on provider-block config, and signs out once on plugin shutdown.

Changes:

  • New providers/utils/session.go with InitSharedAuth / ShutdownSharedAuth / ResetSharedAuthForTest; removed the old Authenticate / SignOut / DeleteAssetByID / SignInCount / AuthMu machinery from methods.go.
  • Both providers' Configure paths now call InitSharedAuth with a deterministic cache key; SDKv2 stores a new providerMeta{authObj, signAppin} instead of a raw *AuthenticationObj, and all per-handler Authenticate/SignOut wrappers in resources, data sources, and ephemerals are deleted. main.go calls ShutdownSharedAuth after tf5server.Serve returns and before any log.Fatal.
  • Tests rewritten: new TestInitSharedAuth_* / TestShutdownSharedAuth_* cases; SDKv2 provider config tests now run against a TLS mock signin server and call ResetSharedAuthForTest; SDKv2 resource tests updated to wrap authenticate in &providerMeta{authObj: …}.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
providers/utils/session.go New shared session lifecycle (init/shutdown/reset) keyed by config hash.
providers/utils/methods.go Removed old refcount-based Authenticate, SignOut, DeleteAssetByID, and associated globals.
providers/utils/methods_test.go Replaced old auth/signout/delete tests with InitSharedAuth / ShutdownSharedAuth coverage.
main.go Calls utils.ShutdownSharedAuth() before any log.Fatal after tf5server.Serve returns.
providers/provider_sdkv2/provider.go providerConfigure builds cache key and uses InitSharedAuth; returns *providerMeta.
providers/provider_sdkv2/common.go Removed authenticate / signOut wrappers; introduced providerMeta.
providers/provider_sdkv2/{folders,safes,managed_account,secrets}.go Updated to use *providerMeta and dropped per-call signin/signout.
providers/provider_sdkv2/provider_test.go Added TLS mock signin server and ResetSharedAuthForTest calls.
providers/provider_sdkv2/{folders_safes,managed_account,secrets}_test.go Updated resource tests to wrap authenticate in &providerMeta{authObj: …}.
providers/provider_framework/provider.go Configure uses InitSharedAuth with same cache key shape as SDKv2.
providers/provider_framework/assets_resource.go Inlined assets.NewAssetObj + DeleteAssetById in delete paths; removed getAssetObj signin/signout.
providers/provider_framework/managed_systems_by_database_resource.go Removed APISignOut helper and signin/signout in getManagedSystemObj.
providers/provider_framework/managed_systems_by_{asset,workgroup}_resource.go Dropped APISignOut and per-handler auth/signout.
providers/provider_framework/{databases,functional_accounts,workgroups}_resource.go Dropped per-handler utils.Authenticate and utils.SignOut.
providers/provider_framework/{managed_account,secrets}_ephemeral.go Dropped per-handler signin/signout.
providers/provider_framework/*_datasource.go (assets, databases, folders, functional_accounts, managed_account, managed_system, platforms, safes, workgroups) Dropped per-read utils.Authenticate and deferred utils.SignOut.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread providers/utils/session.go Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2026

Automation tests Checked

Automation tests failed

Please check the Automation tests run here

@github-actions
Copy link
Copy Markdown

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/BeyondTrust/terraform-provider-passwordsafe 0.00% (ø)
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework 0.00% (ø)
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2 0.00% (ø)
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/utils 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/BeyondTrust/terraform-provider-passwordsafe/main.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/assets_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/assets_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/databases_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/databases_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/folders_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/functional_accounts_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/functional_accounts_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_account_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_account_ephemeral.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_system_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_systems_by_asset_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_systems_by_database_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/managed_systems_by_workgroup_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/platforms_datasource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/provider.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/safes_datasources.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/secrets_ephemeral.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/workgroups_datasources.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_framework/workgroups_resource.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/common.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/folders.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/managed_account.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/provider.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/safes.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/secrets.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/utils/methods.go 0.00% (ø) 0 0 0
github.com/BeyondTrust/terraform-provider-passwordsafe/providers/utils/session.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/folders_safes_test.go
  • github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/managed_account_test.go
  • github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/provider_test.go
  • github.com/BeyondTrust/terraform-provider-passwordsafe/providers/provider_sdkv2/secrets_test.go
  • github.com/BeyondTrust/terraform-provider-passwordsafe/providers/utils/methods_test.go

@btfhernandez btfhernandez marked this pull request as ready for review May 29, 2026 14:06
Copilot AI review requested due to automatic review settings May 29, 2026 14:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

@btfhernandez btfhernandez merged commit 0fb5c3c into main Jun 1, 2026
22 of 24 checks passed
@btfhernandez btfhernandez deleted the refactor/BIPS-38082-share-authenticationObj-across-muxed-providers branch June 1, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants